Skip to content

Conversation

YHallouard
Copy link

Description

Hi everyone,

I am opening this PR to propose an implementation of Soft NMS based on that paper.

I propose a method with_soft_nms, but I'm not sure that it is in the philosophy of Detection.. Would you have a preference? A boolean in with_nms?

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How has this change been tested, please provide a testcase or example of how you tested the change?

I used the same test case than box nms et mask nms.

Any specific deployment considerations

For example, documentation changes, usability, usage/costs, secrets, etc.

Docs

  • Docs updated? What were the changes:

@CLAassistant
Copy link

CLAassistant commented Oct 27, 2024

CLA assistant check
All committers have signed the CLA.

@LinasKo
Copy link
Contributor

LinasKo commented Oct 30, 2024

Hi @YHallouard 👋

Unfortunately, this will take some time to review. To speed things up, would you have some time to do some of the following?

  1. Ideally, yes - we'd like a bool flag on with_nms.
  2. Could you please explain how soft NMS differs from the standard algorithm?
  3. Would it be simple to adapt the current algorithm by adding a boolean flag? If so, let's do that.
  4. Could you please provide a Colab that shows the difference in results? Ideally, it should show improvements in the filtered detections.

Meanwhile, I'll accept this as Hacktoberfest-approved. The PR brings the improved algo to our attention, provides the code, a few tests. Hearing what tradeoffs and design considerations were made helps us a lot too! 🤝

@LinasKo LinasKo added the hacktoberfest-accepted Contribute to the notion of open-source this October! label Oct 30, 2024
@YHallouard
Copy link
Author

YHallouard commented Oct 31, 2024

Hi @YHallouard 👋

Unfortunately, this will take some time to review. To speed things up, would you have some time to do some of the following?

  1. Ideally, yes - we'd like a bool flag on with_nms.
  2. Could you please explain how soft NMS differs from the standard algorithm?
  3. Would it be simple to adapt the current algorithm by adding a boolean flag? If so, let's do that.
  4. Could you please provide a Colab that shows the difference in results? Ideally, it should show improvements in the filtered detections.

Meanwhile, I'll accept this as Hacktoberfest-approved. The PR brings the improved algo to our attention, provides the code, a few tests. Hearing what tradeoffs and design considerations were made helps us a lot too! 🤝

Hi @LinasKo, And thank you very much for your review :)

this will take some time to review.

Yes I know haha, this is why I din't started the documentation, I wanted to get your feedback on implementation first.

  1. Ideally, yes - we'd like a bool flag on with_nms.

Yes, I deeply thought it was the mindset. My concerns was that I need a sigma parameter also, that will be useful only in the case where the bool flag is True. If you have any idea about that, I'll take it :D

  1. Could you please explain how soft NMS differs from the standard algorithm?

Of course, unlike NMS which removes predictions if they are below a certain IOU threshold, Soft-NMS updates the confidence score of each prediction with a continuous function of the IOU. You can then remove them later with a confidence threshold but the main idea is to preserve overlapping bounding boxes with a good confidence.

  1. Could you please provide a Colab that shows the difference in results? Ideally, it should show improvements in the filtered detections.

I'll do that as soon as possible, maybe this weekend :)

Thank for your review again

@YHallouard
Copy link
Author

YHallouard commented Nov 3, 2024

Hi @LinasKo

  1. Could you please provide a Colab that shows the difference in results? Ideally, it should show improvements in the filtered detections.

Here it is :) Soft-NMS vs NMS

@LinasKo
Copy link
Contributor

LinasKo commented Nov 3, 2024

Thanks @YHallouard !

I'll have a look at the start of next week

@YHallouard
Copy link
Author

Hi @LinasKo, sorry for the message, did you had the time to look at the notebook ? :)

@LinasKo
Copy link
Contributor

LinasKo commented Dec 3, 2024

Hi @YHallouard

Unforunately not. It's on my backlog, but honestly, supervision might steal all the time I have this year.

I'd still like to give it a proper review, so lets see - maybe I can make it happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Contribute to the notion of open-source this October!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants